Skip to content

P0 Workbench Reports: verified GVAA DO→DA labels, q/t details and fail-closed type binding - #228

Closed
masarray wants to merge 17 commits into
mainfrom
feat/workbench-p0-verified-report-types-20261017
Closed

masarray wants to merge 17 commits into
mainfrom
feat/workbench-p0-verified-report-types-20261017

Conversation

@masarray

Copy link
Copy Markdown
Owner

Architecture / references

Implements the safe subset of #226 after reviewing masarray/ARIEC61850 MmsSemanticReportValueProjector.cs and masarray/arsas NativeIec61850Client.SemanticReporting.cs / SchemaSafeAggregateProjectionService.cs. Rejects ARIEC61850's legacy MmsDataValueRenderer.GuessFieldNames() positional/child-count guesses.

Implementation

  • A bounded, read-only verified MMS GVAA TypeSpecification index tied to the report association's discovery lifetime and exact domain/item identity, with strict prefix-component match and ambiguity rejection.
  • Structured report member expansion uses actual named/type-ordered GVAA nodes only after nested structure kind and exact child cardinality match. Each received typed stVal, q, t, origin.orCat, etc. is projected with read-only name + bounded value. Existing shared typed q/t evidence decoder supplies validity/time-quality details. Unknown/array/mismatch/untrusted evidence always falls back to existing positional Field N (no false semantic authority).
  • For canonical Browser-supplied discovery, perform at most eight distinct LN-root metadata GVAA requests for the operator-selected DataSet, once per association, before enabling the RCB. Never per-DA Read, duplicate parser, second association, or polling. If a server rejects GVAA, stop probing without blocking RCB activation.
  • The latest report frame only requests detailed typed values; historical data only retains existing bounded summary. QML displays “Verified MMS type · exact field order” versus “Positional · not SCL-labelled” and expands received q/t underneath exact named leaf.
  • C++ integration QA exercises exact schema, q/t typed evidence, wrong kind/length, wrong domain, duplicated root authority, absent schema and truthful fallback; Browser convergence regression guards the owner/performance contract.

Not claimed

This does not yet decode Dbpos human-readable states or IEC SCL EnumType/unit/multiplier unless an exact semantic type is available; never infer from 2-bit payload alone. Full SCL-driven enum and physical paired IEDScout parity remain in #226. No hardware or Windows-installed QA in this PR.

Gates

Exact-head Qt build/55-stage regression, Product Naming Guard, no unresolved Codex review findings. Merge only after validation and post-merge release CI.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-17T09:41:45.647010Z 80bd900 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6247d67274

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/ied_simulator/src/BrowserReportMemberProjection.hpp
Comment thread apps/ied_simulator/src/ied_browser_large_model_qa.cpp Outdated
Comment thread apps/ied_simulator/src/MmsReportController.cpp Outdated
Comment thread apps/ied_simulator/src/BrowserVerifiedReportTypes.hpp

Copy link
Copy Markdown
Owner Author

@codex review

Request exact-head review of 638feba06f97ff86af7cf7905318b3b0ba54793e. Addressed 4 initial findings: current Browser convergence assertion, owning QA evidence vector (no dangling pointers), GVAA prime on both authored static/dynamic flows, explicit verified leaf truncation signal/UI. Please also examine fail-closed findSchema, worker index lifetime across reconnect, and any protocol/latency hazards before approval.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 638feba06f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/ied_simulator/qml/BrowserReportPane.qml
Comment thread apps/ied_simulator/src/MmsReportController.cpp Outdated

Copy link
Copy Markdown
Owner Author

@codex review

Please verify exact HEAD f0285cf39d9bf096b7e03d919c70bcbdaa159c55. Fixed P1 stale guard for dual q/t inspector (both under selected Loader) and P2 association-lifetime GVAA probe/type memory cap of 16 distinct LN roots in addition to 8 per selected DataSet. Prior findings already resolved. Review performance, fail-closed schema behavior, and native QA test safety.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0285cf39d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/ied_simulator/src/MmsReportController.cpp
Comment thread apps/ied_simulator/src/ied_browser_large_model_qa.cpp Outdated

Copy link
Copy Markdown
Owner Author

@codex review

Latest exact HEAD 80bd9003b459bcd6fa09ccce02363c6fc8572108: refreshed idle receive-poll regression literals/ordering to match the typed snapshot args; guarded empty QA report details before accessing first. All previously reported P1/P2 review threads are resolved. Please verify no other stale test contracts, source-lifetime defects, or regressions before merge.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 80bd9003b4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Owner Author

Historical PR reconciliation / archival — 2026-10-24

Disposition: CLOSE AS SUPERSEDED OLD SOURCE, NOT MERGED, AND NOT A FEATURE ACCEPTANCE PASS. Verified historical head: 80bd9003b459bcd6fa09ccce02363c6fc8572108. This source diverges substantially from the current integrated main (M3.H Process Bus tested baseline already merged through #299).

The distinct acceptance remains: GVAA-based exact DO/DA and q/t details without positional guessing. Canonical engineering owner: #226; migration index #308.

All commits and review notes remain reachable from this closed PR. A new narrowly scoped PR must branch from current main, first identify genuinely missing behavior and then pass targeted Qt/MMS/schema/GUI tests. Do not bulk-merge the historical branch, silently lose unknown MMS type safety, or claim physical IED acceptance from old CI. No M3.H Stable asset is modified by this closure.

@masarray masarray closed this Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant